Move recalculate off the main thread and add durable backfill#207
Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughDurably に backfill ジョブを追加し、CLI から Durably 経由でバッチ実行するよう切替。upsert ワーカー削除と runUpsertInWorker の撤去、SQLite ビジーイベント収集の導入、WAL チェックポイント挙動と WAL ジャーナルサイズ制限の調整、バッチ処理のチャンク/イールド改善を実装。 Changes
Sequence Diagram(s)sequenceDiagram
participant CLI as CLI/Command
participant Durably as Durably Instance
participant Job as Backfill Job
participant Org as Organization Service
participant Repo as Repository Handler
participant Worker as Worker Process
CLI->>Durably: triggerAndWait(organizationId, files)
Durably->>Job: run(step, input)
Job->>Org: getOrganization(organizationId)
Org-->>Job: organization data
Job->>Job: validate token
loop each repository
Job->>Repo: step.run(backfillRepo) with progress
Repo->>Worker: analyze(..., onSqliteBusy)
Worker-->>Repo: analyze result + busy events
Repo->>Worker: upsert(...) %% upsert runs via DB path after refactor
Worker-->>Repo: upsert result
Repo-->>Job: repository processed
Job->>Job: update progress
end
Job-->>Durably: return { repositoryCount }
Durably-->>CLI: success with repositoryCount
CLI->>Durably: stop()
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
app/services/jobs/shared-steps.server.ts (1)
196-199: busy イベントのログ出力レベルについて確認してください。SQLite busy イベントが発生した場合、
step.log.warnで警告として出力されています。これはオペレーターへの通知として適切ですが、gaveUpイベントが含まれる場合は、より深刻な問題を示している可能性があります。
gaveUpが 0 より大きい場合はerrorレベルでログ出力することを検討してください。♻️ gaveUp がある場合は error レベルで出力する提案
const busySummary = summarizeBusyEvents(sqliteBusyEvents) if (busySummary) { - step.log.warn(busySummary) + const hasGaveUp = sqliteBusyEvents.some((e) => e.gaveUp) + if (hasGaveUp) { + step.log.error(busySummary) + } else { + step.log.warn(busySummary) + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/services/jobs/shared-steps.server.ts` around lines 196 - 199, The current code logs the summarized busy events with step.log.warn unconditionally; update the handling after calling summarizeBusyEvents(sqliteBusyEvents) so that if the returned busySummary indicates gaveUp > 0 (inspect busySummary.gaveUp) you call step.log.error with the busySummary, otherwise keep step.log.warn; ensure you still only log when busySummary is truthy and reference the summarizeBusyEvents/sqliteBusyEvents variables and the step.log.warn/error calls when making the change.app/services/jobs/backfill.server.ts (1)
37-42: リポジトリ単位のエラーハンドリングを検討してください。現在の実装では、1つのリポジトリで
backfillRepoが失敗すると、ジョブ全体が停止します。他のリポジトリの処理を継続し、失敗したリポジトリのみを後で再試行できるようにする方が、大規模な組織では有用かもしれません。Durably がステップ失敗時にリトライするため、現在の設計でも問題ありませんが、部分的な成功を許容するかどうかは要件次第です。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/services/jobs/backfill.server.ts` around lines 37 - 42, Wrap the per-repo call to backfillRepo inside the step.run callback with a try/catch so a failure for one repository doesn't bubble out and stop the whole job: inside the loop that calls step.run(`backfill:${repoLabel}`...) catch any error from backfillRepo(repository, ...) and log it with the repoLabel and error details, push the repository identifier into a failedRepos array declared outside the loop for later retries, and do not rethrow so the loop continues; optionally, after the loop finishes, check failedRepos and decide whether to surface a job-level error or schedule retries.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@app/services/jobs/backfill.server.ts`:
- Around line 37-42: Wrap the per-repo call to backfillRepo inside the step.run
callback with a try/catch so a failure for one repository doesn't bubble out and
stop the whole job: inside the loop that calls
step.run(`backfill:${repoLabel}`...) catch any error from
backfillRepo(repository, ...) and log it with the repoLabel and error details,
push the repository identifier into a failedRepos array declared outside the
loop for later retries, and do not rethrow so the loop continues; optionally,
after the loop finishes, check failedRepos and decide whether to surface a
job-level error or schedule retries.
In `@app/services/jobs/shared-steps.server.ts`:
- Around line 196-199: The current code logs the summarized busy events with
step.log.warn unconditionally; update the handling after calling
summarizeBusyEvents(sqliteBusyEvents) so that if the returned busySummary
indicates gaveUp > 0 (inspect busySummary.gaveUp) you call step.log.error with
the busySummary, otherwise keep step.log.warn; ensure you still only log when
busySummary is truthy and reference the summarizeBusyEvents/sqliteBusyEvents
variables and the step.log.warn/error calls when making the change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 4997b8fd-425d-4eb1-975f-cc86a03125f0
📒 Files selected for processing (5)
app/services/durably.server.tsapp/services/jobs/backfill.server.tsapp/services/jobs/shared-steps.server.tsbatch/cli.tsbatch/commands/backfill.ts
Summary
recalculateanalyze/upsert work in worker threads so synchronous SQLite work does not block the main threadbusy_timeout, worker retries, step timing logs, and sqlite-busy summariesbackfilljob and switch the CLI command to trigger it through durablyValidation
node --import tsx batch/cli.ts recalculate aScaf_RvvFiOJWD3m8X32DATABASE_URL="file:/Users/coji/progs/upflow/data/data.db" NODE_ENV=production node --import tsx batch/cli.ts recalculate aScaf_RvvFiOJWD3m8X32node --import tsx batch/cli.ts backfill aScaf_RvvFiOJWD3m8X32 --filesnode --import tsx batch/cli.ts recalculate aScaf_RvvFiOJWD3m8X32after backfill completedpnpm validatepassed locallySummary by CodeRabbit
リリースノート
New Features
Chores
Bug Fixes / Reliability
Documentation